feat(control-plane): deliver a bootstrap secret into hosted tenant containers by JSONbored · Pull Request #8253 · JSONbored/loopover

JSONbored · 2026-07-23T14:50:23Z

Summary

Resolves #8202 for its ORB scope (AMS split into #8246, see Boundaries below).

control-plane/src/secret-driver.ts could custody a tenant's secret in the broker but had no way to actually deliver it into a running container -- its own header called this "separate, not-yet-built infrastructure." This PR builds that delivery mechanism.

Why not push the real secret value? I read the real @cloudflare/containers SDK (not just this repo's own hand-rolled test interface) before picking a mechanism. createTenantContainer calls stub.start() exactly once, guarded by isProvisioned() -- there's no second call for injectSecrets to hook into as the issue's push option suggested. And a repeat start() against an already-running/starting container is a no-op or throws at the SDK level (envVars are only ever applied at an actual cold boot). So a literal "push the DB connection string through a second stub.start() call" isn't just worse than pull, it doesn't reliably work at all.

What this ships instead (pull, bootstrapped by a small reorder):

provisionTenant now runs provisionDatabase -> injectSecrets -> createContainer (was createContainer first) so a bootstrap credential exists before the container's one real cold-boot start() call.
injectTenantSecrets (secret-driver.ts) stops discarding the one-time exchange secret the broker already hands back on custody -- it's returned as bootstrapSecret and threaded by provisionTenant into the very next createContainer call.
createTenantContainer (container-driver.ts) delivers it as LOOPOVER_TENANT_SECRET_TOKEN, merged alongside the existing LOOPOVER_PINNED_VERSION into one envVars object. Product-agnostic naming (no ORB_/AMS_ prefix), matching PINNED_VERSION_ENV_VAR's own precedent -- both OrbTenantContainer and AmsTenantContainer will read the identical var name.
New fetchBrokeredStoredSecret (src/orb/broker-client.ts) lets a container exchange that bootstrap credential against the broker's already-wired POST /v1/orb/token stored-secret path (brokerOrbToken's ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL branch, resolveStoredSecret -- already shipped server-side, just never had a client). OrbTenantContainer runs the unmodified self-host image, so ORB gets this for free once wired; AMS has no broker-aware code today, hence the split.

deprovisionTenant's own order is deliberately left unchanged -- "revoke the secret before the DB/container it belonged to is gone" is an independent security property, not a mechanical requirement to mirror provision's new order.

Tests

test/integration/orb-broker.test.ts: new test proving a revoked tenant_db_credential enrollment's secret can no longer be exchanged -- the exact "revocation actually removes access, not just broker custody" deliverable Deliver secrets/env vars into a running hosted tenant container (OrbTenantContainer/AmsTenantContainer) #8202 called for. (The existing revocation test used the GitHub-token enrollment type; this is the missing stored-secret combination.)
test/unit/orb-broker-client.test.ts: full coverage of fetchBrokeredStoredSecret (success, custom broker URL, shared URL-safety validation, non-OK response, missing secretValue, both ?? defensive branches).
control-plane/test/container-driver.test.ts: bootstrap-secret-only, both-pinned-and-bootstrap-merged, neither-set (byte-identical to pre-Build a fleet rollout mechanism #4898), and idempotent-repeat-create cases.
control-plane/test/provisioning.test.ts: new step-order assertion, plus a test proving bootstrapSecret actually threads from injectSecrets' result into the createContainer request (and an omission test for when the driver returns none).
control-plane/test/secret-driver.test.ts, driver-factory.test.ts: updated to expect the now-returned bootstrapSecret.
control-plane/test/provisioning-pagerduty.test.ts: fixed a real bug the reorder surfaced -- its driverThatThrowsOn helper's no-op stand-in for injectSecrets returned undefined (a Promise<void> slipping past injectSecrets' real return type via TypeScript's void-return special-casing). Under the old create-container-first order this was never exercised for createContainer-throws tests; the new order runs injectSecrets first, and its no-op return value gets destructured. Fixed to return a real {}.

Verified: full unsharded npm run test:coverage (20975/20996 passing, 0 failures), tsc --noEmit --incremental false clean, control-plane's own build + node:test suite (193/193) + real c8/lcov coverage (100% line+branch on every file this PR touches), git diff --check clean.

Boundaries

Scoped to the delivery mechanism + ORB, matching #8202's own explicit "needs a design decision, not a default extension" framing -- picking a mechanism was the hard part, and AMS's container-side wiring is genuinely new code (packages/loopover-miner has zero broker-aware code today), not a second call site for the same thing. Split into #8246 rather than bundled here.

Does not wire a runtime consumer for the resolved secret value into src/server.ts's boot sequence -- #8202's own deliverables are the delivery mechanism and proof it works (including revocation), not runtime consumption. That's #7858's hosted half, already listed as blocked-by #8202.

Test plan

npm run typecheck (forced non-incremental)
npm run test:coverage (full unsharded suite)
control-plane: npm run build && npm run test:node (193/193)
control-plane: real coverage via npm run control-plane:coverage -- 100% line+branch on every touched file
git diff --check

…ntainers (#8202) Resolves push-vs-pull: a second stub.start() call from injectSecrets can't reliably deliver a live secret (Cloudflare Containers only apply envVars at an actual cold boot, and createContainer already owns the tenant's one real start() call). Instead, provisionTenant now runs database -> secrets -> container, so injectSecrets' one-time exchange secret (previously discarded) rides the container's own cold-boot envVars as a bootstrap credential; the container exchanges it for the real custodied value via the new fetchBrokeredStoredSecret client against the broker's already-wired stored-secret path. Scoped to the mechanism + ORB (which reuses its unmodified self-host broker-client code for free); AMS's container-side wiring is a real separate lift and follow-up issue #8246.

superagent-security · 2026-07-23T14:51:25Z

Superagent didn't find any vulnerabilities or security issues in this PR.

superagent-security · 2026-07-23T14:51:34Z

Superagent didn't find any vulnerabilities or security issues in this PR.

codecov · 2026-07-23T14:57:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.60%. Comparing base (a0e791d) to head (d8cbbd4).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8253      +/-   ##
==========================================
- Coverage   92.12%   91.60%   -0.53%     
==========================================
  Files         782      782              
  Lines       78531    78585      +54     
  Branches    23720    23728       +8     
==========================================
- Hits        72347    71986     -361     
- Misses       5062     5524     +462     
+ Partials     1122     1075      -47

Flag	Coverage Δ
control-plane	`99.85% <100.00%> (+<0.01%)`	⬆️
shard-1	`59.36% <0.00%> (-0.59%)`	⬇️
shard-2	`46.64% <0.00%> (-0.57%)`	⬇️
shard-3	`52.85% <100.00%> (-0.67%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
control-plane/src/container-driver.ts	`100.00% <100.00%> (ø)`
control-plane/src/index.ts	`100.00% <100.00%> (ø)`
control-plane/src/provisioning.ts	`100.00% <100.00%> (ø)`
control-plane/src/secret-driver.ts	`100.00% <100.00%> (ø)`
control-plane/src/tenant-provisioning-driver.ts	`100.00% <100.00%> (ø)`
src/orb/broker-client.ts	`99.21% <100.00%> (+0.06%)`	⬆️

... and 3 files with indirect coverage changes

loopover-orb · 2026-07-23T15:05:03Z

Warning

⏸️ LoopOver review result - manual review recommended

_{Review updated: 2026-07-23 15:06:10 UTC}

13 files · 1 AI reviewer · 1 blocker · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR wires a real delivery path for a tenant's bootstrap secret into its container's cold-boot env, reordering provisionTenant to database→injectSecrets→createContainer so the secret exists before the one real stub.start() call, and adds fetchBrokeredStoredSecret for the container's own exchange. The mechanism itself is sound and well-reasoned (correctly avoids a second stub.start() call, which the PR author verified against the real SDK doesn't reapply envVars), and test coverage is thorough for the new branches. However, the reorder introduces a genuine orphaned-secret regression: if createContainer fails after injectSecrets has already succeeded, the enrollment's secretRef is silently dropped instead of surfaced for cleanup/revocation.

Blockers

control-plane/src/provisioning.ts:107-118 (provisionTenant): `secretRef` is assigned inside the try block right after `injectSecrets` succeeds, but if the subsequent `createContainer` call throws (a real path — Cloudflare quota errors, transient container-API failures), control jumps straight to the catch/pageAndRethrow and the function never reaches its `return { ...secretRef }` line, so the broker enrollment that was just created is never handed back to the caller to persist and later revoke — it's now a permanently un-revocable dangling secret in the broker, a regression introduced specifically by moving injectSecrets ahead of createContainer; `pageAndRethrow`'s alert (provisioning.ts:69-71) also doesn't include `secretRef`, so even the PagerDuty page gives an operator nothing to manually revoke by.

Nits — 5 non-blocking

control-plane/src/secret-driver.ts:77-79 and broker-client.ts:100 call it a 'one-time' bootstrap credential, but the exchange itself isn't single-use (only revocation invalidates it, per the integration test at test/integration/orb-broker.test.ts:369-376) — worth softening the terminology so a future reader doesn't assume the broker enforces single-use.
No test exercises the createContainer-fails-after-injectSecrets-succeeds path this reorder newly creates (control-plane/test/provisioning.test.ts only covers the happy threading case and the pagerduty tests only fail one step at a time with every other step a no-op) — worth adding one that asserts what happens to the already-injected secretRef.
Thread `secretRef` (not just the raw error) into `pageAndRethrow`/the PagerDuty alert when a failure happens after `injectSecrets` has already run, so an operator has something to revoke manually.
Consider having `provisionTenant`'s catch block attempt a best-effort `revokeSecrets` when `secretRef` is set and a later step failed, mirroring the existing `onFailure` best-effort pattern, rather than leaving custody stranded in the broker.
Possible secret-shaped assignment in the diff (generic_secret_assignment) — Verify the value is not a real credential.

Concerns raised — review before merging

control-plane/src/provisioning.ts:107-118 (provisionTenant): `secretRef` is assigned inside the try block right after `injectSecrets` succeeds, but if the subsequent `createContainer` call throws (a real path — Cloudflare quota errors, transient container-API failures), control jumps straight to the catch/pageAndRethrow and the function never reaches its `return { ...secretRef }` line, so the broker enrollment that was just created is never handed back to the caller to persist and later revoke — it's now a permanently un-revocable dangling secret in the broker, a regression introduced specifically by moving injectSecrets ahead of createContainer; `pageAndRethrow`'s alert (provisioning.ts:69-71) also doesn't include `secretRef`, so even the PagerDuty page gives an operator nothing to manually revoke by.

📋 Copy for AI agents — paste into your coding agent

Fix the following blocker(s) from this PR review:

1. control-plane/src/provisioning.ts:107-118 \(provisionTenant\): \`secretRef\` is assigned inside the try block right after \`injectSecrets\` succeeds, but if the subsequent \`createContainer\` call throws \(a real path — Cloudflare quota errors, transient container-API failures\), control jumps straight to the catch/pageAndRethrow and the function never reaches its \`return \{ ...secretRef \}\` line, so the broker enrollment that was just created is never handed back to the caller to persist and later revoke — it's now a permanently un-revocable dangling secret in the broker, a regression introduced specifically by moving injectSecrets ahead of createContainer; \`pageAndRethrow\`'s alert \(provisioning.ts:69-71\) also doesn't include \`secretRef\`, so even the PagerDuty page gives an operator nothing to manually revoke by.

Decision drivers

❌ Code review — 1 blocker (1 reviewer)
❌ Gate result — Blocking (Repo-configured hard blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#8202
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 14 registered-repo PR(s), 14 merged, 255 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 14 PR(s), 255 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: significant

Linked issue satisfaction

Partially addressed
The PR builds a real pull-based delivery mechanism (bootstrap secret via LOOPOVER_TENANT_SECRET_TOKEN, reordered provisionTenant, fetchBrokeredStoredSecret, and revocation tests), directly satisfying the issue's push/pull design ask and the revocation deliverable for the ORB path, with the container-driver code itself already product-agnostic (bindingFor branches on product, not the env var naming

Review context

Author: JSONbored
Role context: owner (maintainer lane)
Public audience mode: oss maintainer
Lane context: Repository is configured for direct PR review.
Public profile languages: Python, TypeScript, Ruby, Go, MDX, Shell, Solidity, JavaScript
Official Gittensor activity: 14 PR(s), 255 issue(s).
PR-specific overlap: none found.

Contributor next steps

Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.

Signal definitions

Related work = same linked issue, overlapping active PRs, or title/path similarity.
Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

@loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
@loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

Re-run LoopOver review

… bootstrap (#8246) (#8263) Adds tenant-credential-resolution.ts, a duplicated (not imported -- cross- package import from packages/loopover-miner into root src/ fails tsc with TS6059, confirmed) exchange of LOOPOVER_TENANT_SECRET_TOKEN against the broker, mirroring src/orb/broker-client.ts's fetchBrokeredStoredSecret for ORB. hosted-entry.ts resolves it once per wake, best-effort -- no code in this package consumes the resolved value yet (the miner's stores are unconditionally local SQLite), so this proves the mechanism is wired for AMS without making a scheduled cycle fragile against an unused value, mirroring #8202/#8253's own "prove it works, defer consumption" precedent for ORB.

JSONbored mentioned this pull request Jul 23, 2026

AMS container-side wiring for the hosted tenant-secret bootstrap mechanism #8246

Closed

3 tasks

loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 23, 2026

loopover-orb Bot assigned JSONbored Jul 23, 2026

loopover-orb Bot added the manual-review Gittensor contributor context label Jul 23, 2026

JSONbored merged commit 151bad8 into main Jul 23, 2026
14 checks passed

JSONbored deleted the feat/hosted-tenant-secret-bootstrap branch July 23, 2026 15:08

This was referenced Jul 23, 2026

fix(control-plane): revoke orphaned secrets when createContainer fails after injectSecrets #8255

Merged

feat(miner): wire AMS's hosted entry point to the #8202 tenant-secret bootstrap #8263

Merged

github-actions Bot mentioned this pull request Jul 23, 2026

chore(release): cut orb-v3.3.0 #8268

Merged

Navigation Menu

Uh oh!

feat(control-plane): deliver a bootstrap secret into hosted tenant containers - #8253

feat(control-plane): deliver a bootstrap secret into hosted tenant containers#8253
JSONbored merged 1 commit into
mainfrom
feat/hosted-tenant-secret-bootstrap

JSONbored commented Jul 23, 2026

Uh oh!

superagent-security Bot commented Jul 23, 2026

Uh oh!

superagent-security Bot commented Jul 23, 2026

Uh oh!

codecov Bot commented Jul 23, 2026 •

edited

Loading

Uh oh!

loopover-orb Bot commented Jul 23, 2026 •

edited

Loading

⏸️ LoopOver review result - manual review recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JSONbored commented Jul 23, 2026

Summary

Tests

Boundaries

Test plan

Uh oh!

superagent-security Bot commented Jul 23, 2026

Uh oh!

superagent-security Bot commented Jul 23, 2026

Uh oh!

codecov Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

loopover-orb Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏸️ LoopOver review result - manual review recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Jul 23, 2026 •

edited

Loading

loopover-orb Bot commented Jul 23, 2026 •

edited

Loading